F-1370 : Tighten key_len check from >= to ==#10122
Conversation
There was a problem hiding this comment.
Pull request overview
Tightens wc_SignatureGetSize() key-length validation to require exact structure size matches, reducing the risk of mis-typed void* keys being dereferenced, and updates tests/docs to reflect the stricter contract.
Changes:
- Enforce
key_len == sizeof(ecc_key)/key_len == sizeof(RsaKey)inwc_SignatureGetSize(). - Add/adjust API tests for
key_lenoff-by-one and zero-length cases (ECC/RSA). - Update Doxygen to document the exact-size requirement and caller type-safety responsibility.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| wolfcrypt/src/signature.c | Tightens ECC/RSA key_len checks from >= to == and adds explanatory comments. |
| tests/api/test_signature.h | Fixes test registration to include RSA test instead of duplicating ECC. |
| tests/api/test_signature.c | Adds negative tests for key_len being 0, sizeof-1, and sizeof+1 for ECC and RSA. |
| doc/dox_comments/header_files/signature.h | Updates Doxygen for key/key_len constraints and clarifies error behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
retest this please |
48338d3 to
0e14849
Compare
|
retest this please |
1 similar comment
|
retest this please |
dgarske
left a comment
There was a problem hiding this comment.
🐺 Skoll Code Review
Overall recommendation: APPROVE
Findings: 2 total — 2 posted, 1 skipped
Posted findings
- [Low] Capitalization typo in doxygen
key_lendescription —doc/dox_comments/header_files/signature.h:18 - [Medium] Japanese doxygen header not updated to match English documentation —
doc/dox_comments/header_files-ja/signature.h
Skipped findings
- [Medium] Japanese doxygen header not updated to match English documentation
Review generated by Skoll via openclaw
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
8fd94fb to
3c73ef1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3c73ef1 to
e04fe0c
Compare
Description
Summary
wc_SignatureGetSize()(and callers that propagatekey_len) previously accepted anykey_len >= sizeof(key_type), meaning an oversized value silently passed the guard and allowed thevoid*to be dereferenced as the wrong type. This PR tightens the check to exact equality and aligns documentation and tests accordingly.Changes
wolfcrypt/src/signature.ckey_len >= sizeof(ecc_key)→key_len == sizeof(ecc_key)and the equivalent RSA check.void*API cannot verify the actual runtime type of the pointed-to object.doc/dox_comments/header_files/signature.h\param keyfor all seven Signature API functions to document the exact type requirement and the caller's responsibility for passing the correct concrete type.\param key_lento state "Must be exactlysizeof(ecc_key)orsizeof(RsaKey)" (previously said "Size of the key structure", which implied>=semantics).\returnforwc_SignatureGetSize()to document that akey_lenmismatch returnsBAD_FUNC_ARG.Testing
key_len = sizeof(ecc_key) - 1andkey_len = sizeof(ecc_key) + 1intest_wc_SignatureGetSize_ecc.key_len = sizeof(RsaKey) - 1andkey_len = sizeof(RsaKey) + 1intest_wc_SignatureGetSize_rsa.Checklist